Ubshm transport fix 2 - #58

…uickDev#33)

…inQuickDev#57)

Make the whole ubring shared-memory module follow the current brpc
conventions, addressing review feedback:

- Replace the module-local LIKELY/UNLIKELY aliases with BAIDU_LIKELY/
  BAIDU_UNLIKELY and drop the aliases from common.h.
- Use BAIDU_SCOPED_LOCK instead of the local GNU cleanup-attribute
  LOCK_GUARD. The pthread_mutex_t members keep their type, allocation
  and lifetime, so no static-init or placement-new change is needed.
- Use butil::atomic instead of std::atomic. The timer handle slots and
  the trx cleanup slot are now real butil::atomic objects, so plain
  storage is never reinterpreted as an atomic: the timer API takes
  butil::atomic<UbrTimerId>* and UbrTrx/UbrCleanupCtl hold atomic
  handles. UbrTrx slots are value-initialized with placement new so
  those atomics are constructed, replacing the per-acquisition memset.
- Drop the two per-node INFO logs on the shm cleanup retry path in
  UbsShmCallback; DeleteShmToList still records each drained node.

No behavioral change: the removed aliases expanded 1:1, BAIDU_SCOPED_LOCK
locks the same pthread_mutex_t, butil::atomic is layout-identical to
std::atomic, and placement-new value-initialization zeroes exactly what
memset zeroed while also constructing the atomic members.

Address the ubring lifecycle review on the bthread-timer refactor.

Generation validation:
- UbrTimerStart/UbrTimerTask carry an opaque `gen' and pass it back to the
  callback on every fire, so a callback armed for an object that was later
  released and reused can detect that it is stale. `gen' is required and sits
  before the optional backoff; the UBS shm timer passes 0 explicitly.
- UbrScheduleClearTimer takes expect_ubr_id, rejects a mismatch before
  creating the cleanup ctl, and stamps ctl->ubr_id with it instead of reading
  the slot's current generation.
- UbrTrxCallbackCheck runs before the generation check in the close and
  heartbeat callbacks, so trx is null-checked before trx->ubr_id is
  dereferenced; UbrTrxClose and UbEventCallback pass the generation they
  observed for the slot.
- UbrPassiveClearTrx/ClearTrxResource/UbrAddAsynClearTimer thread
  expect_ubr_id through.

Teardown:
- ReleaseUbrTrxFromMgr disarms the per-acquisition close/heartbeat timers
  after the generation check and before the slot is marked FREE, so a
  rollback no longer orphans a periodic task onto the slot's next occupant.
- UbrMgrFini snapshots the USED slots under g_ubr_trx_mgr_mtx and
  UbrTimerDelAndWait's their timers outside the lock before the pool is
  freed; the previous loop only waited for cleanup-ctl callbacks, so a
  dispatched close/heartbeat callback could still be reading UbrTrx when
  FREE_PTR(trx_mgr) ran. The walk is guarded by "all four pool arrays are
  allocated", which also stops the ctl loops from reading arrays that
  UbrMgrInit's allocation-failure path leaves uninitialized.
- UbrClearResourceCheck/UbrPassiveClearTrx stop the per-trx timers through
  UbrStopTrxTimer, which waits with UbrTimerDelAndWait unless the caller is
  the timer callback itself (in_timer_callback), so teardown from a user
  thread or from the ubsmem fault callback cannot clear a trx whose callback
  is mid-flight. UbrTrxCloseCallback re-validates its event queue pointers
  after the generation check, like UbrTrxHBCallback, so a concurrent release
  cannot turn the dereference into a null access.

The ordering argument is a synchronization one: bthread dispatches every
timer callback from a single global timer thread, so two per-trx callbacks
never run concurrently and the one-shot delayed-clear callback is ordered
after them; ub_flying_io_timeout_s only delays the clear so in-flight IO can
drain. Non-blocking UbrTimerDel is kept on the paths that hold
g_ubr_trx_mgr_mtx (ReleaseUbrTrxFromMgr and the callback side), where the
callbacks take the same lock and a wait would deadlock.

Tested: cmake --build cmake-build-debug --target brpc_ubring_unittest, and
./test/brpc_ubring_unittest (12/12 passed).

…inQuickDev#62)

Address the second ubring review round on the bthread-timer refactor.

Timer facade:
- A periodic callback that deletes its own timer now only marks the task
  stopped and leaves it anchored in the handle slot until the callback
  returns; UbrTimerOnFire retires the slot on exit. Previously the callback
  pulled the task out of the slot first, so a concurrent UbrTimerDelAndWait
  returned immediately and the caller could clear a trx whose callback was
  still running.
- UbrTimerDelAndWait recognizes that the caller is the callback of that very
  timer and degrades to the non-blocking delete, which removes the
  in_timer_callback argument that every teardown entry point had to thread
  correctly (and could get wrong).
- UbrTimerStart (one-shot) and UbrTimerStartPeriodic are now separate
  functions, and a periodic start rejects interval_us == 0 instead of
  silently arming a one-shot: that dropped the close check, the heartbeat or
  the shm drain for the rest of the connection's life. The close and hb call
  sites use the periodic form and validate their flags; the shm drain period
  is derived to stay positive because its flag legitimately accepts 0 for the
  one-shot delayed-clear.
- A back-off function returning 0 keeps the previous interval: re-arming for
  "now" would let one timer monopolize the process-wide timer thread.
- The two join waits poll every 100us instead of 1ms; they only run on
  teardown paths.

Cleanup-control publication and trx close:
- UBRingManager::TryClaimTrxClose validates the slot generation and moves
  close_cnt in one critical section, so a close started from a stale snapshot
  cannot mark the slot's new occupant CLOSED or delete its timers.
  UbEventCallback looks the faulty link up before it closes it, so this was
  reachable through the ubsmem fault callback; UbrTrxCloseCheck takes
  expect_ubr_id and is now only a wrapper around the claim.
- TryPublishUnitCleanupCtl publishes the cleanup control object on the trx and
  anchors it in the pool slot under one lock, and DetachUnitCleanupCtl clears
  both, so a force close never observes a trx-side publication without its
  anchor. The inline fallback gates the cleanup on IsUbrTrxSlotUsed instead of
  ubr_id: ReleaseUbrTrxFromMgr does not change ubr_id, so the old check let a
  released slot be cleaned twice.

Shutdown:
- UbrMgrFini takes a shutdown barrier before anything else: acquisitions,
  timer arming (ArmTimersExclusive, now used by UbrAddTimer) and cleanup
  publications are refused, and the faulty-shm callback registers through
  BeginPoolAccess, which Fini waits for before the pool is freed. Because
  arming is serialized by the same lock, the timers armed before the flag are
  the complete set and one wait pass replaces the retry loop whose window was
  never actually closed.
- The shm list is torn down through a handle: DestroyShmTimer takes ShmList**
  and clears it, so g_shm_list is never left dangling, a repeated teardown is
  a no-op and UbsShmFini is idempotent, which makes a repeated ShmMgrFini safe
  instead of a double free.

UbrMgrInit value-initializes the pool instead of memset'ing it, matching the
placement-new UbrTrx construction the style commit introduced (UbrTrx holds
butil::atomic members).

Tested: cmake --build cmake-build-debug --target brpc_ubring_unittest, and
./test/brpc_ubring_unittest (12/12 passed).

* fix(ubshm): retire the slot storage before a deleted one-shot timer returns

UbrTimerDel wins the handle-slot exchange and reports success for a one-shot
timer, but a wrapper that the timer thread already dispatched may still be
between its dispatch and its own (losing) slot exchange. The caller of
UbrTimerDel treats return 0 as "the callback will not run" and frees the
object that stores the slot -- UbrCleanupCtl on the delayed-clear path -- so
that late wrapper access is a use-after-free.

Track the slot retirement on the task: the one-shot wrapper sets slot_retired
right after its slot CAS and before running the user callback, and UbrTimerDel
waits for it when bthread_timer_del reports the task as dispatched. Callers
that win the cancellation get the same guarantee from the delete itself. The
wait only spins on a task-internal flag, so it is safe to do under the trx
manager lock that the force-close, pool-acquire and UbrMgrFini callers hold,
and it is limited to the one-shot path (a periodic task's slot storage is pool
memory that the non-blocking callers never free).

* fix(ubshm): claim forced cleanup and cleanup publication atomically

The force close used to snapshot the pool slot's cleanup control object under
the manager lock and then re-verify "still no ctl" out of the lock. A
concurrent SDK-fault callback could publish a delayed cleanup for the same
generation in between, so both the force close and the delayed-clear callback
would run the cleanup of the same shared memory (double unmap / stale event
queues).

Replace the two steps with ClaimTrxCleanupForced, which snapshots the ctl and,
when there is none, claims the cleanup for the generation inside one critical
section, and make TryPublishUnitCleanupCtl refuse to publish once that claim
happened (new per-acquisition trx->cleanup_forced flag, reset by the next
AcquireUbrTrxFromMgr). Publication and claim are now totally ordered by the
manager lock, so exactly one of them owns the cleanup. An existing publication
keeps the current ctl->state arbitration untouched.

* fix(ubshm): offload ubs sdk cleanup from the global timer thread

Timers are bthread based, so every callback in the process runs on one global
timer thread. UbsShmCallback and the delayed-clear callbacks still called the
UBS SDK there (ubsmem_shmem_unmap/deallocate), and those calls can wait on the
SDK daemon with no latency bound, so a slow or failed shm release delayed every
other timer in the process -- RPC timeouts, backup requests, bthread_usleep
wakeups and ubring's own heartbeat/close checks.

Add a lazily started cleanup worker bthread with an MPSC job queue. The timer
callbacks now only arbitrate cleanup ownership and post a job (shared-memory
drain or trx cleanup); the worker runs the blocking part, and takes over the
timer/callback reference of the cleanup control object. Callers run the work
inline only when the worker cannot be started. The drain keeps the previous
retry semantics (the node stays in the list on unmap failure and the periodic
timer retries it) and at most one drain step is in flight, and DestroyShmTimer
drains the worker before it frees the list and finalizes the SDK. UbrMgrFini
already waits for the control-object references, so it needs no change.

* fix(ubshm): serialize cleanup worker posting with the lazy start

Post() was not serialized with the lazy start of the worker bthread. Two
posters could decide "the worker is started" and "enqueue the job"
independently: one thread won the start CAS and was about to see
bthread_start_background fail, while another read g_worker_started == true and
enqueued a job. Since g_worker_unavailable is terminal and no worker ever runs,
that job is never dequeued -- its UbrCleanupCtl reference leaks, the pool slot
is never released, and g_cleanup_inflight never returns to zero, so every later
UbrCleanupWorker::DrainAndWait (DestroyShmTimer on a teardown path) hangs
forever.

Take a small mutex in Post() around the unavailable check, the start attempt
and the enqueue, so a job can only ever be queued behind a worker that really
started. Posting only happens on cleanup paths, the worker never takes the
mutex, and a failed start now returns false (callers keep running the work
inline) without having enqueued anything.

* style(ubshm): prefix the cleanup claim results with UBR_CLEANUP_CLAIM

The UbrCleanupClaim members were the only constants in ub_ring_manager.h
without the module's UBR_ prefix, next to UBR_MGR_UNIT_* and UBR_CLEANUP_*.
Rename them to UBR_CLEANUP_CLAIM_NOT_OURS / _HAS_CTL / _OWNED_NULL.

* docs(ubshm): document the cleanup worker and the one-shot delete contract

The Timer Management sections still implied all cleanup work runs on the timer
thread. Document that the delayed-clear and pending-unmap callbacks only
arbitrate ownership and post a job to the cleanup worker, and that a one-shot
UbrTimerDel returning 0 now also guarantees the facade is done with the storage
the handle slot lives in, so the caller may free the object holding it
(UbrCleanupCtl on the delayed-clear path).

Also refresh the ordering comment above UbrTrxCloseCallback: with the cleanup
body on the worker, the guarantee rests on the generation gate the worker
re-applies under the manager lock (IsUbrTrxSlotUsed) plus the per-trx
UbrStopTrxTimer/UbrTimerDelAndWait waits, not on the timer thread serializing
the cleanup itself.